Skip to content

List models using open ai api #1234

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 20 commits into from
Jul 31, 2025
Merged

List models using open ai api #1234

merged 20 commits into from
Jul 31, 2025

Conversation

agrimk
Copy link
Member

@agrimk agrimk commented Jul 22, 2025

Description

  • Add a new handler in AQUA SDK to call /predict/v1/models and return the deployed model list.
  • this is called from AQUA UI via the following format -
    BASEURL/aqua/deployments/models/list/<md_ocid>

@oracle-contributor-agreement oracle-contributor-agreement bot added the OCA Verified All contributors have signed the Oracle Contributor Agreement. label Jul 22, 2025
Copy link

📌 Cov diff with main:

Coverage-0%

📌 Overall coverage:

Coverage-18.59%

Copy link

📌 Cov diff with main:

Coverage-0%

📌 Overall coverage:

Coverage-18.59%

Copy link

📌 Cov diff with main:

Coverage-25%

📌 Overall coverage:

Coverage-58.47%

Copy link

📌 Cov diff with main:

Coverage-21%

📌 Overall coverage:

Coverage-58.46%

Copy link
Member

@VipulMascarenhas VipulMascarenhas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added some minor comments

@@ -582,6 +582,19 @@ def embeddings(
payload = {**(payload or {}), "input": input}
return self._request(payload=payload, headers=headers)

def list_models(self) -> Union[Dict[str, Any], Iterator[Mapping[str, Any]]]:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the logic in this method here seems generic, should we call it fetch_data or get_json_response instead oflist_models? Also, docstrings need to be changed here.

self.set_header("Content-Type", "application/json")
endpoint: str = ""
model_deployment = AquaDeploymentApp().get(model_deployment_id)
if model_deployment.endpoint.endswith("/"):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we can simplify this with something like:
endpoint = model_deployment.endpoint.rstrip("/") + "/predict/v1/models"

Copy link

📌 Cov diff with main:

Coverage-23%

📌 Overall coverage:

Coverage-58.47%

Copy link
Member

@VipulMascarenhas VipulMascarenhas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm 👍
there are a few failing tests, can you check them?

Copy link

📌 Cov diff with main:

Coverage-23%

📌 Overall coverage:

Coverage-58.46%

Copy link

📌 Cov diff with main:

Coverage-23%

📌 Overall coverage:

Coverage-58.47%

@agrimk agrimk merged commit e9aaef2 into main Jul 31, 2025
24 checks passed
@agrimk agrimk deleted the list_models_using_openAI_API branch July 31, 2025 12:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
OCA Verified All contributors have signed the Oracle Contributor Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants